Skip to content

feat: drive redirection for the viewer, .rdp files and Windows hotplug - #2178

Merged
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
masterfrom
claude/viewer-drive-redirection-755e63
Oct 22, 2026
Merged

Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
masterfrom
claude/viewer-drive-redirection-755e63

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

Drive redirection used to be reachable only through ActiveX and the daemon's fixed volume list. This makes it usable from the viewer and from standard .rdp files, and adds automatic drive hotplug on Windows.

Changes

  • ironrdp-cfg: DriveRedirection parses drivestoredirect with mstsc semantics (* = every drive including later ones, DynamicDrives, C:\;E:\). A nonempty drivestoredirect wins; otherwise redirectdrives:i:1 selects every drive, as in FreeRDP. New PropertySetExt accessors for both keys plus drive_redirection().
  • ironrdp-client: RdpdrConfig::drives and ConfigBuilder::with_drive_redirection, filled from the property set. The client only carries the selection; frontends build the native factory.
  • ironrdp-rdpdr-native (Windows):
    • A redirected root may now be a directory on a volume (C:\Users\name), not only C:\. Paths with .., \?\/device/UNC prefixes, : or NUL are rejected, and the root is still opened with OBJ_DONT_REPARSE.
    • WindowsRdpdrBackendFactory::with_logical_drives registers drive letters as device IDs 1–26 and announces the selected letters present when each connection (including reconnects) starts.
    • LogicalDriveWatcher watches WM_DEVICECHANGE on a hidden top-level window, plus a 2 s GetLogicalDrives poll for network drives, which are not broadcast. It reports arrivals and removals; arrivals are added to the shared selection.
  • ironrdp-viewer: --drive NAME,PATH, --drives, --home-drive, --dynamic-drives; honors the .rdp keys; attaches the RDPDR backend whenever any drive, smartcard or printer redirection is requested; forwards watcher changes as AddRdpdrDrive / RemoveRdpdrDrive.
  • ironrdp-daemon: honors drivestoredirect / redirectdrives on connect (an explicit ironrdp_rdpdr:i:0 disables them) and keeps a watcher for the session's lifetime. Configured daemon volumes now use device IDs from 27, so they cannot collide with drive letters.

Testing

  • cargo xtask check fmt -v and cargo xtask check lints -v pass.
  • New tests: .rdp parsing and precedence (testsuite-core/tests/cfg.rs), viewer CLI and .rdp mapping (testsuite-extra/tests/client/config.rs), root path validation, logical-drive factory, daemon factory.
  • Opt-in watcher_reports_subst_drive_arrival_and_removal (--ignored) maps a free letter with subst and passes locally: arrival and removal are both reported.
  • Not done: a live session against a server, and a physical USB stick.
  • e2e::egfx_moves_onto_the_udp_tunnel_with_soft_sync timed out once under a parallel run and passed 3/3 alone; it is unrelated to this change.

Merge note

This overlaps the printer redirection PR #2177 in ironrdp-rdpdr-native (factory.rs, exports, README), ironrdp-client config, and the viewer's cli.rs / main.rs. Whichever lands second needs a rebase. The device ID layout is: 0 smartcard, 1–26 drive letters, 27+ named drives, printers from u32::MAX - 1 down.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 21, 2026 00:41
@github-actions github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny scope/cross-cutting Spans multiple architectural boundaries size/XXL Size: 1300 or more counted lines or 50 or more files labels Oct 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Root-path and gateway-policy bypasses remain, alongside hotplug delivery and readiness issues.

5 open issues
What changed in this PR

Adds drive redirection to the viewer and .rdp configuration, backed by Windows drive discovery and hotplug support.

Changes:

  • Parses drive selections and exposes viewer CLI options.
  • Supports directory roots, stable drive-letter IDs, and Windows hotplug notifications.
  • Integrates daemon sessions and adds tests and documentation.
File Description
crates/​ironrdp-viewer/​src/​main.rs Attaches drive backend and retains watcher.
crates/​ironrdp-viewer/​src/​lib.rs Exports Windows drive support.
crates/​ironrdp-viewer/​src/​drives.rs Builds factories and forwards hotplug events.
crates/​ironrdp-viewer/​src/​cli.rs Adds drive selection options.
crates/​ironrdp-viewer/​README.md Documents drive options and properties.
crates/​ironrdp-testsuite-extra/​tests/​client/​config.rs Tests viewer drive configuration.
crates/​ironrdp-testsuite-core/​tests/​cfg.rs Tests parsing, precedence, and serialization.
crates/​ironrdp-rdpdr-native/​src/​windows/​mod.rs Exports drive APIs.
crates/​ironrdp-rdpdr-native/​src/​windows/​logical_drives.rs Implements discovery, selection, and hotplug watching.
crates/​ironrdp-rdpdr-native/​src/​windows/​handles.rs Accepts directory roots and validates paths.
crates/​ironrdp-rdpdr-native/​src/​windows/​factory.rs Registers and selects logical drives.
crates/​ironrdp-rdpdr-native/​src/​lib.rs Exposes Windows drive APIs publicly.
crates/​ironrdp-rdpdr-native/​README.md Documents roots and hotplug integration.
crates/​ironrdp-rdpdr-native/​Cargo.toml Enables required Windows APIs.
crates/​ironrdp-daemon/​src/​daemon.rs Integrates selections, watchers, and reserved IDs.
crates/​ironrdp-daemon/​README.md Documents connect-time drive selection.
crates/​ironrdp-client/​src/​rdp.rs Updates test configuration literals.
crates/​ironrdp-client/​src/​config.rs Carries drive selection through configuration.
crates/​ironrdp-cfg/​src/​lib.rs Adds drive parsing and property accessors.

Review effort: Balanced

Comment thread crates/ironrdp-rdpdr-native/src/windows/handles.rs
Comment thread crates/ironrdp-viewer/src/drives.rs Outdated
Comment thread crates/ironrdp-rdpdr-native/src/windows/logical_drives.rs Outdated
Comment thread crates/ironrdp-rdpdr-native/src/windows/logical_drives.rs Outdated
Comment thread crates/ironrdp-viewer/src/drives.rs Outdated
@github-actions github-actions Bot added automation-failed Exact-head automated classification or review failed or was unavailable risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny and removed risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny labels Oct 21, 2026
@github-actions github-actions Bot added risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny and removed risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny automation-failed Exact-head automated classification or review failed or was unavailable labels Oct 21, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR makes drive redirection configurable (drivestoredirect/redirectdrives) for the viewer and daemon, adds Windows logical-drive enumeration and hotplug watching, and renumbers daemon volumes to device IDs 27+. Verification confirms the two Windows frontends carry near-verbatim copies of the selection-to-factory wiring and the try_send forwarding callback (with a subtle Full/Closed retry contract), define the reserved named-drive device-ID base 27 twice while the complementary 1-26 letter layout lives in ironrdp-rdpdr-native, and diverge on display-name policy: the viewer rejects drive-letter names while the daemon does not. The daemon also ships a public DriveRedirection::contains_letter helper with no production caller. The claimed watcher startup arrival gap does not hold: in both frontends the watcher's initial snapshot precedes connection setup, so any selected volume present at snapshot time is announced by the factory's initial_drives() (selection intersected with currently …

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

Comment thread crates/ironrdp-daemon/src/daemon.rs Outdated
Comment thread crates/ironrdp-daemon/src/daemon.rs Outdated
Comment thread crates/ironrdp-daemon/src/daemon.rs Outdated
Comment thread crates/ironrdp-cfg/src/lib.rs Outdated
@github-actions github-actions Bot added ai-reviewed/1 One automated review completed needs-author-action The pull request author is the current next actor labels Oct 21, 2026
@github-actions github-actions Bot removed the needs-author-action The pull request author is the current next actor label Oct 21, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-redirection PR adding mstsc-syntax drivestoredirect parsing, viewer/daemon wiring, and a Windows hotplug watcher. The core design is sound: device-ID layout avoids collisions, hotplug additions are gated on drive capability, and the earlier duplication was consolidated in response to review. All six candidates were independently verified against pr-head and are published. Two are questions kept open: a nonempty drivestoredirect unconditionally overrides an explicit redirectdrives:i:0 opt-out (a deliberate, tested but protocol-semantics-sensitive precedence), and the watcher's live behavior against real broadcasts and hardware is unproven. The remaining candidates are a silent drop of unrecognized drivestoredirect entries and three low-severity simplifications (duplicated ineligible computation, single-use helper, fallible usize conversions).

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

Comment thread crates/ironrdp-cfg/src/lib.rs
Comment thread crates/ironrdp-cfg/src/lib.rs
Comment thread crates/ironrdp-rdpdr-native/src/windows/logical_drives.rs
Comment thread crates/ironrdp-rdpdr-native/src/windows/logical_drives.rs Outdated
Comment thread crates/ironrdp-cfg/src/lib.rs Outdated
Comment thread crates/ironrdp-rdpdr-native/src/windows/logical_drives.rs
@github-actions github-actions Bot added ai-reviewed/2 Two automated reviews completed needs-author-action The pull request author is the current next actor and removed ai-reviewed/1 One automated review completed labels Oct 21, 2026
@github-actions github-actions Bot removed the needs-author-action The pull request author is the current next actor label Oct 21, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2178 adds drive redirection for the viewer, .rdp properties and Windows hotplug. The core design is sound: DriveRedirection parsing follows mstsc semantics with documented precedence, the path hardening keeps OBJ_DONT_REPARSE and rejects reparse/UNC/stream forms, device IDs 1-26 for letters and 27+ for named drives are collision-checked, and the drive_hotplug_available gate prevents queued events from adding capability after negotiation. All six validated specialist candidates were independently verified in the head revision and are published: the connect path synchronously probes every selected volume with no timeout, the shared drive wiring lives in the daemon crate's public API with client-session types, --rpc silently drops the new drive flags, a hotplug-watcher startup failure aborts the daemon connect, and two small duplication spots remain in ironrdp-cfg and the daemon's error responses.

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

Comment thread crates/ironrdp-rdpdr-native/src/windows/factory.rs Outdated
Comment thread crates/ironrdp-daemon/src/drives.rs
Comment thread crates/ironrdp-viewer/src/cli.rs
Comment thread crates/ironrdp-daemon/src/daemon.rs Outdated
Comment thread crates/ironrdp-cfg/src/lib.rs
Comment thread crates/ironrdp-daemon/src/daemon.rs
@github-actions github-actions Bot added ai-reviewed/3 Final automated review completed needs-author-action The pull request author is the current next actor and removed ai-reviewed/2 Two automated reviews completed labels Oct 21, 2026
@github-actions github-actions Bot added needs-review A human reviewer is the current next actor and removed needs-author-action The pull request author is the current next actor labels Oct 21, 2026
Marc-André Moreau (mamoreau-devolutions) added a commit that referenced this pull request Oct 21, 2026
)

Windows can send the first IRP for a redirected device just before the
`ServerDeviceAnnounceResponse` that accepts it. `ironrdp-rdpdr` ignored
such IRPs without completing them, so the server waited about 30 seconds
and then retried.

This showed up in a live test of drive redirection (#2178) against a
Windows Server session, after a reconnect. The log shows the order:

```
DeviceIoRequest { device_id: 3, major_function: Create }
WARN Ignoring filesystem IRP before device announcement confirmation device_id=3
ServerDeviceAnnounceResponse { device_id: 3, result_code: STATUS_SUCCESS }   (same millisecond)
... 30 s later ...
DeviceIoRequest { device_id: 3, major_function: Create }
```

## Change

A device awaiting its announcement response now counts as announced, for
both filesystem and printer IRPs (`Rdpdr::device_is_announced`). FreeRDP
behaves the same way: it serves every registered device and only
unregisters one when the server rejects it.

- If the server later rejects the device, the existing rejection path
still removes the drive from the backend, which cancels its pending
IRPs.
- A printer `Create` before acceptance is completed with
`STATUS_UNSUCCESSFUL` in the channel and does not reach the backend. It
would start a native print job that a rejection could leave allocated,
because IRPs for rejected devices, including the close, are ignored.
Other early printer IRPs complete without allocating.
- IRPs for devices that were never announced, or were rejected, are
still ignored, as MS-RDPEFS recommends.

## Testing

- New `ironrdp-rdpdr` tests: a drive IRP before the response reaches the
backend; an early IRP followed by a rejection removes the drive. New
core testsuite tests: a printer IRP before the response is completed, an
early printer `Create` fails without reaching the backend, and a
`Create` after acceptance reaches it. All three fail without the fix.
- `cargo xtask check fmt -v` and `cargo xtask check lints -v` pass, as
do the tests in `ironrdp-rdpdr`, `ironrdp-rdpdr-native`,
`ironrdp-testsuite-core` and `ironrdp-testsuite-extra`.
- The live reconnect has not been rerun with this fix.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added risk/medium Behavioral change that does not substantially alter a core public API triage/overlap Possible overlap with another pull request; advisory only needs-author-action The pull request author is the current next actor and removed needs-review A human reviewer is the current next actor risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny labels Oct 22, 2026
- ironrdp-cfg: parse drivestoredirect with mstsc semantics (`*`,
  DynamicDrives, drive letters) into DriveRedirection; an explicit
  redirectdrives:i:0 disables it, otherwise a nonempty drivestoredirect wins
  and redirectdrives:i:1 alone selects every drive. Report unsupported
  entries.
- ironrdp-client: carry the selection on RdpdrConfig::drives and ignore
  dynamic drive additions on a channel built without drive capability.
- ironrdp-rdpdr-native: allow directory roots below a volume (opened without
  following reparse points, including for root opens), register drive
  letters as device IDs 1-26 and announce the selected mapped letters at each
  connection without probing volumes, and add LogicalDriveWatcher for
  WM_DEVICECHANGE and network-drive hotplug with retried delivery.
- ironrdp-daemon: honor drivestoredirect on connect, keep a hotplug watcher
  for the session, and share the drive wiring with the viewer through
  ironrdp_daemon::drives. Configured volumes use device IDs from 27.
- ironrdp-viewer: add --drive NAME,PATH, --drives, --home-drive and
  --dynamic-drives alongside the port options, and forward hotplugged drives
  to the session.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny needs-review A human reviewer is the current next actor and removed needs-author-action The pull request author is the current next actor risk/medium Behavioral change that does not substantially alter a core public API triage/overlap Possible overlap with another pull request; advisory only labels Oct 22, 2026
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 1de38a2 into master Oct 22, 2026
43 checks passed
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) deleted the claude/viewer-drive-redirection-755e63 branch October 22, 2026 01:33
@github-actions github-actions Bot removed the needs-review A human reviewer is the current next actor label Oct 22, 2026

This branch was previously deployed

1 inactive deployment
llm-providers — c6174345 Deployed Oct 22, 2026 by mamoreau-devolutions via Classify pull request #5844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/3 Final automated review completed breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny scope/cross-cutting Spans multiple architectural boundaries size/XXL Size: 1300 or more counted lines or 50 or more files

Development

Successfully merging this pull request may close these issues.

2 participants